-
Notifications
You must be signed in to change notification settings - Fork 4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
v3 - consultants in project #831
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
@@ -36,9 +36,10 @@ export const customerCaseProjectInfo = defineField({ | |||
defineField({ | |||
// TODO: We should be able to select the consultants from a list | |||
name: "consultants", | |||
description: "The consultants for the project", | |||
description: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to use @variant.no or could we only use "amn"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We are not sure enough that they are unique across countries (e.g. [email protected] and [email protected] could be two different people). So going for the safer option now. The user experience may be a little worse, but that could be fixed with a dropdown/search solution (where the internal value does not matter to the user)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point!
{customerCase.projectInfo.consultants.join(", ")} | ||
</Text> | ||
</div> | ||
{consultantsResult.ok && ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
isn't it enough to write consultantsResult?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you thinking just consultantsResult && ...
?
This is using the Result
type, so consultantsResult
will always be an object. So checking if consultantsResult
is truthy would always return true. The proper check with Result
is to check if .ok
is true.
<p className={styles.consultantName}> | ||
{consultant.name} | ||
</p> | ||
{consultant.officeName && ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's nice to have office location, but we should probably ask designers
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point! I guess it was included as a temporary replacement for role (e.g. "Salg" in Figma), which is not information we have right now.
a919ecc
to
3156025
Compare
3156025
to
dd79b9f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this looks good!
4c077e9
to
d14083b
Compare
since we don't know if aliases are unique across countries
3b121a6
to
feab346
Compare
See #815
Shows the employee cards for each employee in the project consultants list. Data fetched from Chewbacca.
The project consultants field should now contain the employee emails.
Desktop
Mobile